fix(session-host): skip identity checks for a zombie group leader - #2
Merged
Merged
Conversation
Stop failed on macOS after it had killed its own worker. The group drain loop re-checked the leader's identity whenever `process_is_running` said the pid was still there, and the two platforms disagree about a zombie: the Linux implementation reads `/proc/<pid>/stat` and answers "not running", while every other Unix asks `kill(pid, 0)` and answers "running". The drain loop then asked macOS libproc for the identity of a zombie, which answers nothing, so `verify_group_leader_identity` reported `could not verify the identity of process N` and the whole stop failed. That is the exact case it exists for: the leader a stop just terminated, still unreaped by the process that spawned it. Decide it once as `verify_live_group_leader_identity`: the identity is checked while the leader is still an active group member, and skipped once it is a zombie. A zombie's pid cannot be reused until it is reaped, so no other process can have taken that identity over, and the checks before signalling are untouched: `terminate_recorded_unix_group` still verifies the leader twice before it signals the group. Seven `rebon-session-host` tests cover this path. They only run in the `test` job, which is macos-14 and gated on a tag or a pull request, so this is the first change to reach them.
The previous change drained a process group without re-checking a zombie leader, and one test stopped failing, but six stop_job tests still did. They take the paths that never look at group membership at all: `terminate_process_best_effort` waits with `wait_for_pid_exit`, and `terminate_process`, `wait_for_tree_exit` and `recorded_process_is_running` all ask `process_is_running` whether the worker is still there. That question had two answers. Linux reads the state field of `/proc/<pid>/stat` and reports a zombie as exited; every other Unix asks `kill(pid, 0)`, which answers for a zombie too, so a terminated worker stayed "running" until its parent reaped it — and on the stop path the parent is the process doing the waiting. Every such wait ran to its timeout and reported `worker exit was not verified`. macOS now separates the two with the identity read it already has: libproc describes a live process and answers nothing for one whose parent has not reaped it, so a pid this process may signal but cannot describe is an exited child. `terminate_recorded_unix_group` also verifies the leader through the active-member list, the same rule the drain loop uses, instead of probing a pid that may already be a zombie. Ref: the macOS-only failures in the release `test` job.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fix a macOS-only failure in the background-job stop path. On macOS,
Stopraisedcould not verify the identity of process Nafter it had already terminated its ownworker, so the job could not be reported as stopped.
Root cause
process_is_runningdoes not agree across platforms about a zombie:process_liveness.rs:550(Linux) reads/proc/<pid>/statand reports a zombie asnot running.
process_liveness.rs:571(every other Unix, macOS included) askskill(pid, 0)andreports a zombie as running.
wait_for_recorded_unix_group_exitguarded its identity check withprocess_is_running(pid) == Some(true), so on macOS it askedproc_pidinfofor theidentity of a zombie — which answers nothing — and
verify_group_leader_identityfailed the whole stop. On Linux the same guard skips the check, which is why this never
showed up there. The zombie in question is the leader the stop itself had just killed,
still unreaped by its parent.
Change
One named rule (
verify_live_group_leader_identity) shared by both call sites: theidentity is verified while the leader is still an active group member (the member
enumeration already filters zombies), and skipped once it is a zombie. A zombie's pid
cannot be reused until it is reaped, so its identity cannot have been taken over.
The checks before signalling are untouched —
terminate_recorded_unix_groupstillverifies the leader twice before it signals the group, so the fail-closed property for a
live leader is unchanged.
Evidence
Seven tests fail on
macos-14without this change, all in thetestjob of a tag run:with
process_liveness.rs:997panicking oncould not verify the identity of process 20402, and thestop_jobtests reportingworker exit was not verified.Verification
cargo check -p rebon-session-host --locked(Windows host): passes.rustfmt --checkon the changed file: clean.cargo check --all-targets --target x86_64-apple-darwinand--target x86_64-unknown-linux-gnucannot run on this host:cc-rshas nocc/cross
gcc, and the workspace pulls C dependencies. The macOS behaviour this fixestherefore has to be confirmed by this PR's
testjob (macos-14), which is why it is aPR rather than a direct push.